Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

test: assert events emitted from contract #216

Merged
merged 36 commits into from
Sep 13, 2024
Merged

test: assert events emitted from contract #216

merged 36 commits into from
Sep 13, 2024

Conversation

chungquantin
Copy link
Collaborator

@chungquantin chungquantin commented Aug 19, 2024 •
edited
Loading

Link to issue: #181
Refactor fungible integration-tests contract to emit events by default and catch those events in the integration test cases.

@chungquantin chungquantin self-assigned this Aug 19, 2024
@codecov-commenter
Copy link

codecov-commenter commented Aug 19, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 48.89%. Comparing base (7bbc04c) to head (c6d390d).
Report is 2 commits behind head on daan/api.

@@             Coverage Diff              @@
##           daan/api     #216      +/-   ##
============================================
+ Coverage     45.92%   48.89%   +2.97%     
============================================
  Files            47       47              
  Lines          4279     4532     +253     
  Branches       4279     4532     +253     
============================================
+ Hits           1965     2216     +251     
- Misses         2252     2254       +2     
  Partials         62       62

see 10 files with indirect coverage changes

@chungquantin
Copy link
Collaborator Author

@Daanvdplas Could I receive review for this PR?

Daanvdplas
Daanvdplas reviewed Aug 28, 2024
View reviewed changes
Copy link
Collaborator

@Daanvdplas Daanvdplas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good! Left small nitpicks.

This PR is going to daan/api but I'm not sure whether we want this. I say we really focus on getting frank/extension merged and then rebase those small PRs.

pop-api/integration-tests/src/fungibles/mod.rs Outdated Show resolved Hide resolved
pop-api/integration-tests/src/fungibles/mod.rs Outdated Show resolved Hide resolved
pallets/api/src/fungibles/mod.rs Outdated Show resolved Hide resolved
@chungquantin chungquantin mentioned this pull request Sep 3, 2024
Draft
12 tasks
evilrobot-01
evilrobot-01 requested changes Sep 7, 2024
View reviewed changes
Copy link
Collaborator

@evilrobot-01 evilrobot-01 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few things need tweaking I'm afraid: return type of call not checked before emitting events, token terminology throughout (may want to get that PR merged first) and then it needs conflicts resolving.

pallets/api/src/fungibles/mod.rs Outdated Show resolved Hide resolved
pop-api/integration-tests/Cargo.toml Outdated Show resolved Hide resolved
pop-api/integration-tests/contracts/fungibles/lib.rs Show resolved Hide resolved
pop-api/integration-tests/contracts/fungibles/lib.rs Outdated Show resolved Hide resolved
pop-api/integration-tests/src/fungibles/mod.rs Outdated Show resolved Hide resolved
pop-api/integration-tests/src/fungibles/mod.rs Outdated Show resolved Hide resolved
pop-api/integration-tests/src/fungibles/utils.rs Outdated Show resolved Hide resolved
pop-api/integration-tests/src/fungibles/utils.rs Outdated Show resolved Hide resolved
@chungquantin chungquantin changed the base branch from daan/api to daan/refactor-api September 13, 2024 07:41
@chungquantin chungquantin mentioned this pull request Sep 13, 2024
Merged
Daanvdplas
Daanvdplas requested changes Sep 13, 2024
View reviewed changes
pop-api/src/primitives.rs Outdated Show resolved Hide resolved
pop-api/integration-tests/contracts/create_token_in_constructor/lib.rs Outdated Show resolved Hide resolved
pop-api/integration-tests/contracts/fungibles/lib.rs Outdated Show resolved Hide resolved
pop-api/src/v0/fungibles.rs Outdated Show resolved Hide resolved
Base automatically changed from daan/refactor-api to daan/api September 13, 2024 14:50
evilrobot-01
evilrobot-01 approved these changes Sep 13, 2024
View reviewed changes
Copy link
Collaborator

@evilrobot-01 evilrobot-01 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, pleased with the event naming.

@Daanvdplas Daanvdplas merged commit bf27640 into daan/api Sep 13, 2024
10 checks passed
@Daanvdplas Daanvdplas deleted the chungquantin/test-api_events branch September 13, 2024 18:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@peterwht peterwht peterwht approved these changes

@evilrobot-01 evilrobot-01 evilrobot-01 approved these changes

@Daanvdplas Daanvdplas Daanvdplas approved these changes

Assignees

@chungquantin chungquantin

Labels
api Pop API ready-for-final-review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@chungquantin @codecov-commenter @peterwht @evilrobot-01 @Daanvdplas